-
Notifications
You must be signed in to change notification settings - Fork 466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix to remove concatenated media names #6737
Fix to remove concatenated media names #6737
Conversation
5c58780
to
87abb38
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A first pass. Need to see tests showing this functionality working as expected for various formats of media names (non-existent, one binary, a list of one binary, and a list of two or more binaries, at a minimum.
applications/cdr/src/cdr_util.erl
Outdated
@@ -54,3 +59,16 @@ do_save_cdr(AccountMODb, Doc) -> | |||
-spec register_views() -> 'ok'. | |||
register_views() -> | |||
kz_datamgr:register_views_from_folder('cdr'). | |||
|
|||
-spec check_media_names(list()|kz_term:api_binary()) -> list(). | |||
check_media_names(MediaNames) when is_list(MediaNames) -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can do this all in the function head:
check_media_name([HeadMN | TalMN]) ->
Implies the is_list
check, and binds the head and tail of the list, all in one go!
applications/cdr/src/cdr_util.erl
Outdated
re:split(MediaNames, ","). | ||
|
||
-spec check_last_media_name(list())->list(). | ||
check_last_media_name([FirstMediaName | []])-> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[FirstMediaName | []]
is the same as [FirstMediaName]
- the | []
is implied.
-spec check_last_media_name(list())->list(). | ||
check_last_media_name([FirstMediaName | []])-> | ||
re:split(FirstMediaName, ","); | ||
check_last_media_name([FirstMediaName | LasttMediaName])-> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[FirstMediaName | LastMediaName]
means that LastMediaName
will be a list of binaries. Then you re:split
on it below which is likely to crash.
Where are the tests showing this functionality working as expected?
check_media_names(MediaNames) when is_list(MediaNames) -> | ||
[HeadMN|TailMN] = MediaNames, | ||
re:split(HeadMN, ",") ++ check_last_media_name(TailMN); | ||
check_media_names(MediaNames) -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this crash is MediaNames
is 'undefined'
?
3bee0dd
to
e75d14d
Compare
e75d14d
to
6361e05
Compare
No description provided.